Fix int(QuadPrecision) for NaN, Inf, and out-of-int64 values#104
Conversation
SwayamInSync
changed the title
int(QuadPrecision) for NaN, Inf, and out-of-int64 values
| // Route the longdouble backend through quad as as_integer_ratio does; | ||
| // the prior `(long long)longdouble_value` cast also saturated/UBed on | ||
| // NaN/Inf/out-of-range. | ||
| value = Sleef_cast_from_doubleq1((double)self->value.longdouble_value); |
There was a problem hiding this comment.
Both this and the similar cast alluded to in the comment will lose precision on any platform with extended precision native long double.
Instead of doing this, you should write a utility function that checks for NaN and inf with std::isnan and std::isinf, which do support extended precision long double natively and then write the value to a string buffer, which you can parse however you need it.
| assert int(QuadPrecision(str(n))) == n | ||
|
|
||
| @pytest.mark.parametrize("exponent", [40, 60, 80, 100]) | ||
| def test_int_powers_of_two_far_above_int64(self, exponent): |
There was a problem hiding this comment.
This and many other tests in this file aren't parametrized by the backend, which leads to missing the issue I pointed out above.
| - name: Install Intel SDE | ||
| run: | | ||
| curl -o /tmp/sde.tar.xz https://downloadmirror.intel.com/859732/sde-external-9.58.0-2025-06-16-lin.tar.xz | ||
| SDE_URL="https://github.com/SwayamInSync/numpy-quaddtype/releases/download/sde-toolchain/sde-external-10.8.0-2026-03-15-lin.tar.xz" |
There was a problem hiding this comment.
URL isn't working for curl(same in NumPy repo), although interactively works.
Self-hosting from my fork, till some other solution comes in
|
Cool, those were important catches. I fixed them all from complete codebase and its ready for 2nd review round. |
ngoldbaum
left a comment
ngoldbaum
left a comment
There was a problem hiding this comment.
I didn't go over the new test logic in detail but it looks like the issue I spotted in the last round is fixed.
| - name: Install Intel SDE | ||
| run: | | ||
| curl -fSL -o /tmp/sde.tar.xz https://github.com/nihui/ncnn-assets/releases/download/toolchain/sde-external-10.8.0-2026-03-15-lin.tar.xz | ||
| SDE_URL="https://downloadmirror.intel.com/859732/sde-external-9.58.0-2025-06-16-lin.tar.xz" |
There was a problem hiding this comment.
Maybe add a TODO to revert this when Intel's mirror is less flaky?
There was a problem hiding this comment.
Yup, I will keep this in check, right now the binary is hosted from my own fork's release assets, so no 3rd party here.
2 participants
closes #97
AI Disclosure:
Claude wrote the test cases